Surface HttpRequestException from AndroidMessageHandler by Copilot · Pull Request #11682 · dotnet/android

Copilot · 2026-06-17T13:53:02Z

Xamarin.Android.Net.AndroidMessageHandler reported transport/protocol failures as the legacy System.Net.WebException, contradicting the documented HttpClient.SendAsync contract (which throws HttpRequestException).

Changes

AndroidMessageHandler: all WebException throw sites in the request path (timeout, protocol, security, IO, connect, max‑redirections, invalid redirect URI, protocol‑method setup) now throw HttpRequestException.
Back-compat: the original WebException — including its WebExceptionStatus — is kept as the InnerException, so code migrated from classic Xamarin.Android that inspects status (and existing test helpers that walk the inner chain) keeps working. Centralized in one helper:

static HttpRequestException CreateHttpRequestException (string message, Exception? innerException, WebExceptionStatus status)
    => new HttpRequestException (message, new WebException (message, innerException, status, null));

Connection-failure path: ConnectAsync wraps every connection failure in HttpRequestException, so DoProcessRequest now catches that HttpRequestException when its InnerException is a Java.Net.ConnectException, unwraps it, and re-wraps it via CreateHttpRequestException with WebExceptionStatus.ConnectFailure. Previously the catch (Java.Net.ConnectException) block was unreachable, so connection failures did not get the intended inner WebException(ConnectFailure) back-compat wrapping.
Tests: connecting to an unused port asserts HttpRequestException with a preserved inner WebException whose Status == ConnectFailure; exceeding MaxAutomaticRedirections asserts HttpRequestException with a preserved inner WebException whose Status == UnknownError.

Notes

Public API surface is unchanged; only the thrown exception type differs.
Callers catching WebException at the top level will no longer match — this is the intended behavioral change. The WebException remains reachable via ex.InnerException.

…andler Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Copilot

Pull request overview

This PR updates Xamarin.Android.Net.AndroidMessageHandler to align with the HttpClient.SendAsync contract by surfacing request-path transport/protocol failures as HttpRequestException (while preserving the legacy WebException + WebExceptionStatus via InnerException for back-compat).

Changes:

Replace WebException throw sites in the request path with HttpRequestException, preserving a WebException (with status) as the inner exception.
Centralize exception creation in a CreateHttpRequestException(...) helper.
Add tests asserting HttpRequestException is thrown for connection failures and exceeding MaxAutomaticRedirections.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs`	Switches request-path failures to `HttpRequestException` and introduces a helper to preserve legacy `WebException` details.
`tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs`	Adds regression tests to validate the new exception surface and inner-exception preservation expectations.

@@ -839,7 +846,7 @@ protected virtual async Task WriteRequestContentToOutput (HttpRequestMessage req
 				if (Logger.LogNet)
 					Logger.Log (LogLevel.Info, LOG_APP, $"Connection exception {ex}");
 				// Wrap it nicely in a "standard" exception so that it's compatible with HttpClientHandler
-				throw new WebException (ex.Message, ex, WebExceptionStatus.ConnectFailure, null);
+				throw CreateHttpRequestException (ex.Message, ex, WebExceptionStatus.ConnectFailure);


jonathanpeppers · 2026-06-17T20:12:07Z

@copilot address the code review comments left above

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Copilot · 2026-06-17T20:37:08Z

@copilot address the code review comments left above

Addressed all three review comments in 28f733d:

DoProcessRequest now catches the HttpRequestException that ConnectAsync wraps around Java.Net.ConnectException, unwraps it, and re-wraps via CreateHttpRequestException so the inner WebException(ConnectFailure) back-compat path is actually exercised.
ConnectionFailureThrowsHttpRequestException now asserts the inner WebException.Status == ConnectFailure.
ExceedingMaxAutomaticRedirectionsThrowsHttpRequestException now asserts the inner WebException.Status == UnknownError.

simonrozsival · 2026-06-18T12:55:26Z

/review

github-actions · 2026-06-18T12:56:25Z

✅ Android PR Reviewer completed successfully!

github-actions

✅ LGTM

A focused, well-reasoned change. AndroidMessageHandler now surfaces transport/protocol failures as HttpRequestException (per the HttpClient.SendAsync contract) while preserving the legacy WebException — including its WebExceptionStatus — as the InnerException for code migrated from classic Xamarin.Android. I formed an independent assessment of the diff and then reconciled it with the PR description; the claims hold up.

Verified:

All WebException throw sites converted. Every throw site in the request path now routes through the CreateHttpRequestException helper; no bare WebException throws remain. ✓
The previously-dead catch (Java.Net.ConnectException) is now reachable. ConnectAsync wraps every connection failure in HttpRequestException, so the old catch could never match. The new catch (HttpRequestException ex) when (ex.InnerException is Java.Net.ConnectException) correctly unwraps and re-wraps it so the WebException(ConnectFailure) back-compat path actually runs. ✓
Status preservation is correct. WebExceptionStatus.UnknownError for the max-redirections / invalid-redirect-URI paths matches what the old WebException(string) constructor produced, so status-inspecting code is unaffected. ✓
No collateral breakage. No other product code or test asserts a bare WebException from this handler; AssertRejectsRemoteCertificate already catches both types and keeps passing. ✓
Regression tests added for the connect-failure and max-redirections paths, asserting the inner WebException.Status (traced MaxAutomaticRedirections = 1 → throws on the first redirect, so the single-BeginGetContext listener is sufficient). ✓
CI is green — 40/40 checks, including the macOS emulator device-test legs that exercise Mono.Android-Tests. ✓

Severity counts: ❌ 0 · ⚠️ 0 · 💡 2 (both optional/non-blocking, posted inline)

Populate HttpRequestException.HttpRequestError for fuller modern-contract conformance.
The inner-WebException shim only covers Java.Net.ConnectException; DNS/timeout connect failures aren't classified (pre-existing gap, not a regression).

Minor follow-up (pre-existing, not inlineable): the comment at AndroidMessageHandlerTests.cs:308-310 ("...we throw both of these...") and the now-unreachable catch (System.Net.WebException) {} on line 311 are stale after this change — the handler no longer throws a bare WebException. Worth tidying in a follow-up (stale-comment / dead-code cleanup).

Nice work — no blocking issues.

Generated by Android PR Reviewer for issue #11682 · 793.2 AIC · ⌖ 47.2 AIC · ⊞ 37.8K
_{Comment /review to run again}

github-actions · 2026-06-18T13:08:45Z

+		// existing code that migrated from classic Xamarin.Android and still inspects WebException (and its
+		// WebExceptionStatus), we keep the original WebException as the inner exception.
+		static HttpRequestException CreateHttpRequestException (string message, Exception? innerException, WebExceptionStatus status)
+			=> new HttpRequestException (message, new WebException (message, innerException, status, null));


🤖 💡 API design — Optional/non-blocking: since Mono.Android targets modern .NET, consider also populating HttpRequestException.HttpRequestError (added in .NET 8) here, in addition to the inner WebException. The point of this change is to honor the HttpClient.SendAsync contract, and SocketsHttpHandler sets values like HttpRequestError.ConnectionError / SecureConnectionError so callers can branch on ex.HttpRequestError rather than unwrapping the legacy WebException. e.g. for the connect-failure path:

new HttpRequestException (HttpRequestError.ConnectionError, message, new WebException (message, innerException, status, null))

Mapping each WebExceptionStatus to an HttpRequestError is a bit more than a one-liner, so feel free to defer — just flagging it for fuller modern-contract conformance.

Rule: Conform to the modern HttpClient contract

Deferring to a follow-up. Mapping each WebExceptionStatus to an HttpRequestError (e.g. ConnectFailure → ConnectionError, SecureChannelFailure → SecureConnectionError, Timeout → ?, ProtocolError → ?, UnknownError → ?) needs deliberation and its own test coverage, and would expand the scope of this otherwise-focused back-compat PR. Filing a follow-up issue.

github-actions · 2026-06-18T13:08:45Z

 				if (Logger.LogNet)
 					Logger.Log (LogLevel.Info, LOG_APP, $"  connected");
-			} catch (Java.Net.ConnectException ex) {
+			} catch (HttpRequestException ex) when (ex.InnerException is Java.Net.ConnectException connectException) {


🤖 💡 Patterns — Optional/non-blocking: this back-compat shim restores an inner WebException only for Java.Net.ConnectException. Other failures that ConnectAsync wraps the same way — DNS (Java.Net.UnknownHostException → NameResolutionFailure) and connect timeouts (Java.Net.SocketTimeoutException → Timeout) — still surface as HttpRequestException whose InnerException is the raw Java exception, so WebException-walking code (including the IsConnectionFailure test helper) won't classify them. This is a pre-existing gap, not a regression — before this change the old catch (Java.Net.ConnectException) was dead, so no connect failure got the WebException wrapper; this PR strictly improves the most common case. A follow-up could map the DNS/timeout cases too for completeness.

Rule: Preserve back-compat consistently

Agreed — this is a pre-existing gap (the old catch (Java.Net.ConnectException) was dead, so DNS/timeout never got the WebException wrapper either). Out of scope for this PR; tracking in a follow-up issue alongside the HttpRequestError work.

After this PR, AndroidMessageHandler no longer throws bare WebException -- all throw sites route through CreateHttpRequestException, which wraps in HttpRequestException (with WebException as InnerException). The catch (System.Net.WebException) {} in AssertRejectsRemoteCertificate is therefore unreachable, and the comment claiming we throw both is stale. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jonathanpeppers · 2026-06-18T14:16:45Z

Thanks for the review! Addressed the stale-comment / dead-code follow-up in 56138fd: removed the now-unreachable catch (System.Net.WebException) {} and the misleading "we throw both" comment in AssertRejectsRemoteCertificate. Confirmed by grepping throw new WebException in src/Mono.Android/Xamarin.Android.Net/ — zero matches; all 8 throw sites now route through CreateHttpRequestException.

Deferring the two inline 💡 suggestions (HttpRequestError population and DNS/timeout WebException shim) to a follow-up — replied inline.

Initial plan

9333184

Copilot AI assigned Copilot and jonathanpeppers Jun 17, 2026

Copilot started work on behalf of jonathanpeppers June 17, 2026 13:53 View session

Copilot AI and others added 2 commits June 17, 2026 14:22

Throw HttpRequestException instead of WebException in AndroidMessageH…

e9d69b0

…andler Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Add redirect test asserting inner WebException is preserved

47fb418

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix HttpClient throwing WebException instead of HttpRequestException~~ Surface HttpRequestException from AndroidMessageHandler Jun 17, 2026

Copilot finished work on behalf of jonathanpeppers June 17, 2026 14:28

Copilot AI requested a review from jonathanpeppers June 17, 2026 14:28

jonathanpeppers marked this pull request as ready for review June 17, 2026 20:03

jonathanpeppers requested a review from simonrozsival as a code owner June 17, 2026 20:03

Copilot AI review requested due to automatic review settings June 17, 2026 20:03

Copilot started reviewing on behalf of jonathanpeppers June 17, 2026 20:04 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Copilot started work on behalf of jonathanpeppers June 17, 2026 20:12 View session

Catch wrapped ConnectException and assert inner WebException status

28f733d

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>

Copilot finished work on behalf of jonathanpeppers June 17, 2026 20:41

simonrozsival approved these changes Jun 18, 2026

View reviewed changes

github-actions Bot reviewed Jun 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Surface HttpRequestException from AndroidMessageHandler#11682

Surface HttpRequestException from AndroidMessageHandler#11682
Copilot wants to merge 5 commits into
mainfrom
copilot/fix-httpclient-webexception-issue

Copilot AI commented Jun 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers commented Jun 17, 2026

Uh oh!

Copilot AI commented Jun 17, 2026

Uh oh!

simonrozsival commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jun 18, 2026

Uh oh!

jonathanpeppers Jun 18, 2026

Uh oh!

github-actions Bot Jun 18, 2026

Uh oh!

jonathanpeppers Jun 18, 2026

Uh oh!

jonathanpeppers commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Copilot AI commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers commented Jun 17, 2026

Uh oh!

Copilot AI commented Jun 17, 2026

Uh oh!

simonrozsival commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

✅ LGTM

Uh oh!

github-actions Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 18, 2026 •

edited

Loading